Repository navigation
Conversation
|
👋 Thanks for assigning @jkczyz as a reviewer! |
| } | ||
| let max_total_lsp_fee_limit_msat = | ||
| max_total_lsp_fee_limit_msat.unwrap_or(amount_msat).min(amount_msat); | ||
| if min_total_fee_msat > max_total_lsp_fee_limit_msat { |
There was a problem hiding this comment.
This allows fee == amount, but the LSP side doesn't: lightning-liquidity rejects a buy request when opening_fee >= payment_size_msat, and only opens the channel if amt_to_forward_msat > 0.
So fee == amount still goes to the LSP and gets rejected there. Since we don't handle BuyRequestFailed, the user gets LiquidityRequestFailed after the 5s timeout. I think that's what the (1_000, None, Err(LiquidityRequestFailed)) test case is hitting.
Should this be >=? Same for .filter(|fee| *fee <= gross_amount_msat) in event.rs.
There was a problem hiding this comment.
Yes, we're more liberal what we accept here. No, this is intentional == as in theory it could make sense that the client just wants a channel and is fine if the payment all goes to fees. That said, we have higher HTLC minimum values anyways, so in practice we won't ever see a 0-value HTLC coming from the LSP.
| .or_else(|| { | ||
| lsps2_parameters.max_proportional_opening_fee_ppm_msat.and_then(|max_prop_fee| { | ||
| // If it's a variable amount payment, compute the actual fee. | ||
| compute_opening_fee(gross_amount_msat, 0, max_prop_fee) |
There was a problem hiding this comment.
For variable-amount payments this uses 0 as the minimum fee, but the LSP charges max(min_fee_msat, ceil(gross * ppm / 1e6)). We don't store min_fee_msat in LSPS2Parameters, so when the minimum fee is the bigger one, we reject a correctly charged payment, after the LSP has already opened the channel.
With this test's LSP config (1_000 msat min, 10_000 ppm), any variable payment under 100_000 msat would hit this. It was already there before this PR, so a follow-up is fine too.
There was a problem hiding this comment.
Good point, we should finally persist min_fee_msat and use it when checking the opening fees. I believe we did it in another PR already, but that never landed. Will update shortly.
|
Rebased to fix CI. |
21125ab to
23ba291
Compare
Variable-amount JIT invoices retained only a proportional fee limit. When an LSP's minimum fee was larger, the client rejected an otherwise valid payment after the channel had been opened. Retain the negotiated minimum in invoice metadata and use it when validating variable payments. Continue decoding historical payment records that do not carry a minimum. Co-Authored-By: HAL 9000
23ba291 to
bdc6484
Compare
The test decodes metadata with a minimum opening fee using the current reader. Its comment incorrectly claimed to exercise older readers. Describe the encoded TLV fields without claiming compatibility coverage that this test does not provide. Co-Authored-By: HAL 9000
LSPS2 clients could request a fixed-amount invoice whose opening fee exceeded the payment, leaving rejection to the LSP. Variable-amount payments also checked their fee against the net receipt, rejecting otherwise valid payments. Bound fixed-amount fees by the invoice amount even without an application cap. Check variable-amount fees against the gross payment when it arrives, allowing equality while rejecting excessive fees and arithmetic overflow. Co-Authored-By: HAL 9000
The total-fee metadata test did not cover fees above the payment amount. The fixed-amount JIT invoice docs also omitted the payment amount cap. Cover payments below, equal to, and above the total fee limit. Document both fee caps and LiquidityFeeTooHigh for both fixed-amount JIT APIs. Co-Authored-By: HAL 9000
bdc6484 to
985df11
Compare
4 participants
LSPS2 clients could request a fixed-amount invoice whose opening fee exceeded the payment, leaving rejection to the LSP. Variable-amount payments also checked their fee against the net receipt, rejecting otherwise valid payments.
Bound fixed-amount fees by the invoice amount even without an application cap. Check variable-amount fees against the gross payment when it arrives, allowing equality while rejecting excessive fees and arithmetic overflow.
Co-Authored-By: HAL 9000